Skip to content

Collect pod logs for async job tests, since must-gather does not collect the pods log#633

Merged
dbasunag merged 4 commits intoopendatahub-io:mainfrom
dbasunag:test
Sep 24, 2025
Merged

Collect pod logs for async job tests, since must-gather does not collect the pods log#633
dbasunag merged 4 commits intoopendatahub-io:mainfrom
dbasunag:test

Conversation

@dbasunag
Copy link
Copy Markdown
Collaborator

@dbasunag dbasunag commented Sep 23, 2025 •
edited by coderabbitai Bot
Loading

Description

How Has This Been Tested?

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

Summary by CodeRabbit

  • Tests
    • Improved timeout handling in asynchronous upload job tests: when a timeout occurs, the suite now automatically captures pod metadata and per-container logs for easier diagnosis.
    • Consolidated diagnostics are saved to the test artifacts directory, with graceful fallbacks and clear warnings on collection failures.
    • Enhances troubleshooting of intermittent failures without changing runtime behavior or impacting end-user features.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Sep 23, 2025 •
edited
Loading

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Adds timeout handling for upload pod status waits in tests by capturing pod details on TimeoutExpiredError. Introduces collect_pod_information to dump pod YAML and per-container logs to a must-gather or fallback directory. Updates imports to include TimeoutExpiredError and must-gather utilities.

Changes

Cohort / File(s) Summary of changes
Async job utils
tests/model_registry/async_job/utils.py		 Added collect_pod_information(pod) to write pod YAML and container logs to a resolved base dir; on uploadPod.wait_for_status timeout, collect pod info then re-raise TimeoutExpiredError; imported TimeoutExpiredError, get_base_dir, get_must_gather_collector_dir.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0442e1a and 6472a58.

📒 Files selected for processing (1)
  • tests/model_registry/async_job/utils.py (3 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@dbasunag
Copy link
Copy Markdown
Collaborator Author

dbasunag commented Sep 23, 2025

/build-push-pr-image

1 similar comment
@dbasunag
Copy link
Copy Markdown
Collaborator Author

dbasunag commented Sep 23, 2025

/build-push-pr-image

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Sep 23, 2025

Status of building tag pr-633: success.
Status of pushing tag pr-633 to image registry: success.

@dbasunag
Copy link
Copy Markdown
Collaborator Author

dbasunag commented Sep 23, 2025

/build-push-pr-image

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Sep 23, 2025

Status of building tag pr-633: success.
Status of pushing tag pr-633 to image registry: success.

@dbasunag
Copy link
Copy Markdown
Collaborator Author

dbasunag commented Sep 24, 2025

/build-push-pr-image

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Sep 24, 2025

Status of building tag pr-633: success.
Status of pushing tag pr-633 to image registry: success.

fege
fege approved these changes Sep 24, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@fege fege left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@dbasunag dbasunag changed the title Test Collect pod logs for async job tests, since must-gather does not collect the pods log Sep 24, 2025
@dbasunag dbasunag marked this pull request as ready for review September 24, 2025 15:19
@dbasunag dbasunag requested a review from lugi0 as a code owner September 24, 2025 15:19
@dbasunag dbasunag merged commit 4e29be9 into opendatahub-io:main Sep 24, 2025
9 of 10 checks passed
@dbasunag dbasunag deleted the test branch September 24, 2025 15:21
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Sep 24, 2025

Status of building tag latest: success.
Status of pushing tag latest to image registry: success.

sheltoncyril pushed a commit to sheltoncyril/opendatahub-tests that referenced this pull request Oct 2, 2025
@dbasunag @sheltoncyril
Collect pod logs for async job tests, since must-gather does not coll…
8a019af 
…ect the pods log (opendatahub-io#633)

* Remove tests assiciated with default-model-catalog

* temp
mwaykole pushed a commit to mwaykole/opendatahub-tests that referenced this pull request Jan 23, 2026
@dbasunag @mwaykole
Collect pod logs for async job tests, since must-gather does not coll…
21e6615 
…ect the pods log (opendatahub-io#633)

* Remove tests assiciated with default-model-catalog

* temp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fege fege fege approved these changes

@lugi0 lugi0 lugi0 approved these changes

Assignees

No one assigned

Labels

build-push-pr-image lgtm-by-fege lgtm-by-lugi0 ModelRegistry size/s

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dbasunag @fege @lugi0 @rhods-ci-bot